Skip to content

[Pytorch][Common] Hybrid quantization#2817

Open
negvet wants to merge 61 commits into
NVIDIA:mainfrom
negvet:hybrid_quantization
Open

[Pytorch][Common] Hybrid quantization#2817
negvet wants to merge 61 commits into
NVIDIA:mainfrom
negvet:hybrid_quantization

Conversation

@negvet

@negvet negvet commented Mar 31, 2026

Copy link
Copy Markdown
Collaborator

Description

Hybrid (per-direction) quantization. Hybrid means rowwise/colwise can use different formats via CustomRecipe(qfactory).
This is an experimental feature.
The main problem that it tries to solve is that precision requirements are non-uniform.

Current recipes set one format for both rowwise and colwise directions.
Hybrid quantization enables, e.g. MXFP8 fwd and NVFP4 bwd (or vice versa) or any other valid combination. No need for a hardcoded recipe for every combination.

Composer-style (Composer 2 paper) grouped GEMM recipe, e.g. row-scaled NVFP4 fwd + MXFP8 bwd:

# CustomRecipe calls quantization_factory(role) for each quantized tensor
# Factory chooses formats

def hybrid_factory(role):
    is_grouped_linear = role is not None and role.module_type == "grouped_linear"
    is_linear = role is not None and role.module_type == "linear"

    if is_grouped_linear and role.tensor_type == "input":
        return HybridQuantizer(
            rowwise_quantizer=NVFP4Quantizer(row_scaled_nvfp4=True, ...),
            columnwise_quantizer=MXFP8Quantizer(...),
        )

    if is_grouped_linear and role.tensor_type == "weight":
        return HybridQuantizer(
            rowwise_quantizer=NVFP4Quantizer(...),
            columnwise_quantizer=MXFP8Quantizer(...),
        )

    if is_grouped_linear and role.tensor_type == "grad_output":
        return MXFP8Quantizer(...)

    if is_linear:
        return MXFP8Quantizer(...)

    return MXFP8Quantizer(...)

recipe = CustomRecipe(qfactory=hybrid_factory)
with autocast(recipe=recipe):
    y = model(x)

By default, the above factory uses columnwise_source="original", so MXFP8 backward operands are quantized from the original high-precision tensor. Use columnwise_source="rowwise_dequantized" when the backward operand should be derived from the dequantized rowwise NVFP4 forward value.

C++ optimizations (fusions, etc.) will come as standalone PRs. cc @kainzhong

TODO:

  • Convergence of base (non-hybrid) recipes
  • HybridFloat8BlockScaling is xfailed under FSDP2 because dim-0 shards can split 128-row block-scale tiles, producing all-gathered scale buffers whose shape does not match the global tensor.
  • Delayed scaling
  • Mid-training recipe change

Follow-up issue tracker #3158.

Integration

Ecosystem integration (all functional, unit-tested):

  • [Done] quantized_model_init
  • [Done] FSDP2 (TODO: optimize communication buffers)
  • [Done] CPU offloading
  • [Done] Activation recomputation
  • [Done] TP/SP (TODO: enable quantized AG)

Megatron-LM integration status:

  • [Done] 1 GPU baseline
  • [Done] DP + distributed optimizer
  • [TODO] quantized_model_init + --fp{4,8}-param-gather + dist opt (persistent low-precision params via quantized_model_init + sharded-master FP32 → quantized cast via quantize_master_weights.)
    - [Done] Per-tensor Float8 hybrid (delayed and/or current, any per-direction combination
    including same-format, cross-format Float8, single-direction)
    - [TODO] Per-block hybrid sub-quantizers (MXFP8, NVFP4, Float8Blockwise) — each rejected per-direction by quantize_master_weights; unblocker is TE-side cast-helper / kernel.
  • [TODO] Megatron-FSDP + --fp{4,8}-param-gather (fix private attribute access)
  • [TODO] Torch FSDP2 + --fp{4,8}-param-gather
    - [Done] TE-side hybrid FSDP2 path works end-to-end for Float8 / MXFP8 / Float8Blockwise sub-storages (TODO: need some minor MLM update)
    - [TODO] NVFP4 sub-storage FSDP2 hooks
  • [Done] Activation recompute
  • [Done] CPU offload
  • [Done] TP/SP/PP
  • [Done] MoE + EP + grouped GEMM (qwen3 MoE; _hybrid_split_quantize under Megatron MoE)

Review

Total diff +14000
New hybrid source (hybrid_tensor.py, hybrid_tensor_storage.py, identity_tensor.py, identity_tensor_storage.py) ~1800
Adjacent modifications ~1500
Tests are the rest (~10K)

Suggested reading order

  1. Foundation — 7553e6a: Python containers + quantize/gemm dispatch/unwrap
  • tensor/hybrid_tensor.py — HybridQuantizer + HybridQuantizedTensor
    -columnwise_source controls whether columnwise quantization uses the original input or the rowwise-dequantized value.
  • tensor/storage/hybrid_tensor_storage.py
  • cpp_extensions/gemm.py — _unwrap_hybrid_A/B
  • common/transpose/quantize_transpose_square_blockwise.cu - Block FP8 columnwise-only null-checks
  • Module hooks in module/{base,grouped_linear,layernorm_linear,layernorm_mlp}.py
  • Tests: TestHybridQuantizer*, TestHybridGemmBitwiseIdentical* (proves zero-overhead vs vanilla recipes when both formats match), TestHybridDirectionUnwrap*, TestHybridGroupedLinear*

1.1 Identity passthrough — b99277a

  • tensor/identity_tensor.py and tensor/storage/identity_tensor_storage.py — IdentityQuantizer / IdentityTensor high-precision passthrough
  • custom_recipes/quantization_factory_zoo.py — examples for high-precision fwd/bwd directions and columnwise_source="rowwise_dequantized"
  • Tests: test_identity_quantizer.py plus hybrid tests covering Identity inside HybridQuantizer
  1. quantized_model_init + FusedAdam — f80f5d0
  • hybrid_tensor.py::HybridQuantizer.update_quantized — delegates to each sub-quantizer; unblocks workspace-cache quantize_() and FusedAdam writeback
  • module/base.py workspace-cache invalidation
  • Tests: TestHybridQuantizedModelInit, TestHybridFusedAdam, TestHybridQuantizedParamsEndToEnd, TestHybridCheckpoint, TestQuantizedParamsEquivalence*
  1. FSDP2 support — 2185b30
  • New base FSDP2 buffer protocol on QuantizedTensorStorage: fsdp_buffer_fields / fsdp_extract_buffers / fsdp_assign_gathered. Generic, reusable beyond hybrid.
  • Per-format overrides on Float8TensorStorage (direction-aware) and MXFP8TensorStorage (trips/re-applies scale alignment padding around the gather)
  • hybrid_tensor.py::fsdp_pre/post_all_gather + torch_dispatch for the FSDP2 op set (view, split, as_strided, slice, copy_, new_zeros, clone, detach)
  • Non-safety in float8_tensor.py and mxfp8_tensor.py for single-direction sub-storages (columnwise-only on Hopper/L40)
  • Tests: TestHybridTorchDispatchFSDP2Ops, TestHybridFsdpPreAllGatherProtocol, TestHybridFsdpRoundtrip (bitwise-exact against manual all_gather(dequantize(shard))), plus tests/pytorch/distributed/fsdp2_tests/
  1. CPU offloading — 103fffe
  • hybrid_tensor_storage.py::clear() (v1 path) + prepare_for_saving / restore_from_saved chain (v2 path)
  • hybrid_tensor.py::detach() re-wraps each sub-storage via make_like (required by cpu_offload_v2's detach → prepare_for_saving pattern; sharing sub-storage objects would null-out fields on the original)
  • TestHybridCpuOffloadPushPop, plus updates to test_cpu_offloading*.py
  1. Activation recomputation — 16fb371
  • Uses existing QuantizedTensorStorage::prepare_for_saving / restore_from_saved protocol, preserving ordering across both sub-storages
  • Tests: 20 bitwise tests in TestHybridActivationRecompute
  1. TP/SP — a50fd63
  • hybrid_tensor.py::HybridQuantizer.supports_only_rowwise_all_gather — overrides to handle the NVFP4 columnwise-dequantize gap in the BF16 fallback path
  • distributed.py::gather_along_first_dim — hybrid branch re-quantizes with both directions after AG (since hybrid has no _create_transpose synthesis path)
  • Tests: 9 distributed tests in run_hybrid_tp_sp.py / test_hybrid_tp_sp.py
  1. Megatron-LM integration — a164cd3
  • tensor/utils.py::_route_hybrid_to_buckets — per-direction dispatch for quantize_master_weights: iterates both sub-storages, routes each independently into the per-format bucket matching its own sub-quantizer type
  • Hybrid branches in replace_raw_data and post_all_gather_processing
  • Today: per-tensor Float8 sub-quantizers (delayed + current) work in any per-direction combination. Per-block sub-quantizers raise per-direction with in-code TODOs naming the unblocker.
  • Tests: TestHybridQuantizeMasterWeights, TestHybridPostAllGatherProcessing

Type of change

  • Documentation change (change only to the documentation, either a fix or a new content)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Infra/Build change
  • Code refactoring

Changes

Please list the changes introduced in this PR:

  • Change A
  • Change B

Checklist:

  • I have read and followed the contributing guidelines
  • The functionality is complete
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@greptile-apps

greptile-apps Bot commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR introduces hybrid (per-direction) quantization for PyTorch: a HybridQuantizer that composes two existing quantizers (e.g. NVFP4 forward + MXFP8 backward) and a HybridQuantizedTensor that holds rowwise and columnwise data in different formats. An IdentityQuantizer / IdentityTensor passthrough is also added for high-precision directions inside a hybrid. The change spans ~14k lines across quantizers, tensors, GEMM dispatch, FSDP2 all-gather, CPU offloading, TP/SP, and Megatron-LM distopt integration.

  • New tensor types: HybridQuantizedTensor / HybridQuantizedTensorStorage compose two sub-storages; IdentityTensor wraps an unquantized tensor as a first-class quantizer output. Both integrate with the FSDP2, CPU-offload, activation-recompute, and checkpoint protocols.
  • GEMM dispatch: _unwrap_hybrid_A/B in cpp_extensions/gemm.py extract the direction-appropriate sub-storage before the cuBLAS call; _unwrap_identity_tensor materializes the plain tensor for Identity operands.
  • Distopt (quantize_master_weights): _route_hybrid_to_buckets iterates both sub-storages and routes each independently into per-format buckets; _update_transpose_only_float8_flat_fragment handles the Hopper/L40 columnwise-only Float8 case (where _data=None) for the non-FSDP path. The FSDP shard path (use_fsdp_shard_model_weights=True) in _cast_master_weights_to_fp8_current_scaling lacks an equivalent guard and will crash for Hopper columnwise-only hybrid Float8 sub-storages routed from _route_hybrid_to_buckets.

Confidence Score: 4/5

Safe to merge for non-Hopper FSDP+distopt workflows; the Hopper/L40 columnwise-only hybrid Float8 distopt path in _cast_master_weights_to_fp8_current_scaling will crash for affected configurations.

The FSDP non-shard path for Hopper columnwise-only hybrid Float8 distopt has been protected by _update_transpose_only_float8_flat_fragment, but the shard path (use_fsdp_shard_model_weights=True) calls quantizer.update_quantized directly on a Float8Tensor with _data=None, crashing the C++ kernel. This affects Hopper/L40 + distributed optimizer + hybrid Float8 current-scaling — a combination the PR description explicitly marks as done.

transformer_engine/pytorch/tensor/utils.py (_cast_master_weights_to_fp8_current_scaling, lines 561–574): the FSDP shard branch needs the same _update_transpose_only_float8_flat_fragment guard as the non-FSDP branch for Hopper columnwise-only Float8 sub-storages.

Important Files Changed

Filename Overview
transformer_engine/pytorch/tensor/hybrid_tensor.py New HybridQuantizer + HybridQuantizedTensor classes: quantize dispatch, make_empty, update_quantized, fsdp_pre/post_all_gather, torch_dispatch for FSDP2 ops. Solid design with extensive docstrings; the fsdp_post_all_gather _sync_usage call now properly invalidates the Float8 transpose cache.
transformer_engine/pytorch/tensor/storage/hybrid_tensor_storage.py New HybridQuantizedTensorStorage mixin: sub-storage delegation for dequantize, prepare_for_saving/restore_from_saved, fsdp_extract_buffers, view, get_metadata. Logic is correct; previously flagged repr NoneType issue is fixed.
transformer_engine/pytorch/tensor/utils.py Adds _route_hybrid_to_buckets for quantize_master_weights dispatch, _update_transpose_only_float8_flat_fragment for Hopper columnwise FP8 flat-fragment scatter, and _cast_master_weights_to_identity. The Hopper transpose-only guard is correctly applied to the non-FSDP branch but missing from the use_fsdp_shard_model_weights=True branch in _cast_master_weights_to_fp8_current_scaling.
transformer_engine/pytorch/cpp_extensions/gemm.py Adds _unwrap_hybrid_A/B and _unwrap_identity_tensor for transparent hybrid/identity operand extraction before GEMM dispatch. Semantics (T→rowwise, N→columnwise) are correct for TN/NN/NT layouts; missing None-guard when requested sub-storage is absent.
transformer_engine/pytorch/module/grouped_linear.py Adds _hybrid_split_quantize, _split_quantize_with_identity_fallback, _validate_grouped_quantizer_list, and per-generation validation; wires hybrid/identity dispatch in forward and backward. Previously flagged None-in-quantizer-list and columnwise_source bugs are fixed.
transformer_engine/pytorch/tensor/float8_tensor.py Extensive changes to support columnwise-only Float8Tensor (data=None, transpose populated) for Hopper/L40 hybrid sub-storages: view, slice, split, as_strided, new_zeros, clone, reduce_ex, _reset_caches all updated. Shape inference for transpose-only split previously flagged is fixed.
transformer_engine/pytorch/tensor/storage/float8_tensor_storage.py Adds direction-aware fsdp_buffer_fields/fsdp_extract_buffers/fsdp_assign_gathered for columnwise-only Float8 FSDP2 transport (movedim-based M-major layout), and fixes dequantize for transpose-only storage. Previously flagged fsdp_buffer_fields direction-blindness is now fixed.
transformer_engine/pytorch/tensor/storage/mxfp8_tensor_storage.py Adds MXFP8 fsdp_buffer_fields/fsdp_extract_buffers/fsdp_assign_gathered with block-scale alignment stripping/re-padding. Columnwise scale truncation uses floor division (// MXFP8_BLOCK_SCALING_SIZE) — prior review thread noted this was fixed in c7e11d2.
transformer_engine/pytorch/tensor/identity_tensor.py New IdentityQuantizer/IdentityTensor for high-precision passthrough; correctly detaches the held tensor in quantize_impl to prevent spurious autograd edges. Implementation is clean and well-documented.
transformer_engine/pytorch/distributed.py Adds HybridQuantizer branch in gather_along_first_dim that temporarily restores both-direction usage before quantizing the all-gathered BF16 buffer, then restores original flags. Try/finally ensures flag restoration on error.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    HQ[HybridQuantizer\nrowwise_quantizer\ncolumnwise_quantizer\ncolumnwise_source] --> |quantize_impl| HT[HybridQuantizedTensor\nrowwise_storage\ncolumnwise_storage]
    HT --> |_unwrap_hybrid_A/B| RW[RowwiseTensorStorage\nFloat8/MXFP8/NVFP4/Identity]
    HT --> |_unwrap_hybrid_A/B| CW[ColumnwiseTensorStorage\nFloat8/MXFP8/Identity]
    RW --> |_unwrap_identity_tensor| GEMM[general_gemm / grouped_gemm\nC++ kernel]
    CW --> |_unwrap_identity_tensor| GEMM
    HT --> |fsdp_pre_all_gather| FSDP[FSDP2 all-gather\nrow_buffers + col_buffers]
    FSDP --> |fsdp_post_all_gather| HT2[HybridQuantizedTensor\nfull-size param]
    HQ --> |quantize_master_weights| ROUTE[_route_hybrid_to_buckets\nper-direction dispatch]
    ROUTE --> |Float8Quantizer| DSP[delayed_scaling_params]
    ROUTE --> |Float8CurrentScalingQuantizer| CSP[current_scaling_params]
    ROUTE --> |IdentityQuantizer| IDP[identity_params]
    CSP --> |non-FSDP: guarded| CAST1[_update_transpose_only\n_float8_flat_fragment OK]
    CSP --> |FSDP: unguarded| CRASH[quantizer.update_quantized\n_data=None crash on Hopper]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    HQ[HybridQuantizer\nrowwise_quantizer\ncolumnwise_quantizer\ncolumnwise_source] --> |quantize_impl| HT[HybridQuantizedTensor\nrowwise_storage\ncolumnwise_storage]
    HT --> |_unwrap_hybrid_A/B| RW[RowwiseTensorStorage\nFloat8/MXFP8/NVFP4/Identity]
    HT --> |_unwrap_hybrid_A/B| CW[ColumnwiseTensorStorage\nFloat8/MXFP8/Identity]
    RW --> |_unwrap_identity_tensor| GEMM[general_gemm / grouped_gemm\nC++ kernel]
    CW --> |_unwrap_identity_tensor| GEMM
    HT --> |fsdp_pre_all_gather| FSDP[FSDP2 all-gather\nrow_buffers + col_buffers]
    FSDP --> |fsdp_post_all_gather| HT2[HybridQuantizedTensor\nfull-size param]
    HQ --> |quantize_master_weights| ROUTE[_route_hybrid_to_buckets\nper-direction dispatch]
    ROUTE --> |Float8Quantizer| DSP[delayed_scaling_params]
    ROUTE --> |Float8CurrentScalingQuantizer| CSP[current_scaling_params]
    ROUTE --> |IdentityQuantizer| IDP[identity_params]
    CSP --> |non-FSDP: guarded| CAST1[_update_transpose_only\n_float8_flat_fragment OK]
    CSP --> |FSDP: unguarded| CRASH[quantizer.update_quantized\n_data=None crash on Hopper]
Loading

Comments Outside Diff (1)

  1. transformer_engine/pytorch/tensor/utils.py, line 567-574 (link)

    P1 FSDP path skips Hopper columnwise-only guard in current-scaling distopt

    When _route_hybrid_to_buckets routes a Float8CurrentScalingQuantizer columnwise sub-storage to current_scaling_params, the resulting entry has model_weight_fragment = shard_fragment._columnwise_storage — a Float8Tensor with _data=None on Hopper/L40 (where columnwise-only FP8 stores its payload in _transpose). The non-FSDP branch is correctly protected by _update_transpose_only_float8_flat_fragment, but the use_fsdp_shard_model_weights=True path skips straight to quantizer.update_quantized(master_weight, model_weight_fragment). The C++ quantize kernel writes the result to model_weight_fragment._data; with _data=None it will crash. The fix applied to lines 562–565 needs a symmetric guard before line 574 for the FSDP shard branch.

Reviews (26): Last reviewed commit: "[pre-commit.ci] auto fixes from pre-comm..." | Re-trigger Greptile

Comment thread transformer_engine/pytorch/module/grouped_linear.py Outdated
Comment thread transformer_engine/pytorch/tensor/storage/hybrid_tensor_storage.py
Comment thread transformer_engine/pytorch/tensor/hybrid_tensor.py Outdated

@timmoon10 timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I think this moves us in a good direction. I see some minor bugs, as well as bugs reported by @greptile-apps.

Comment on lines +52 to +53
rowwise_result = self.rowwise_quantizer.quantize(tensor)
columnwise_result = self.columnwise_quantizer.quantize(tensor)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we handle the case where not all usages are needed? I'd expect something like:

Suggested change
rowwise_result = self.rowwise_quantizer.quantize(tensor)
columnwise_result = self.columnwise_quantizer.quantize(tensor)
rowwise_result = self.rowwise_quantizer.quantize(tensor) if self.rowwise_usage else None
columnwise_result = self.columnwise_quantizer.quantize(tensor) if self.columnwise_usage else None

@negvet negvet May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4858491

requires_grad: bool = False,
pin_memory: bool = False,
) -> HybridQuantizedTensor:
self.rowwise_quantizer.internal = True

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we just set internal=True in the constructor? I don't think we ever need PyTorch tensor functionality in the per-usage data.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would not work under FSDP2.

Comment thread transformer_engine/pytorch/tensor/hybrid_tensor.py Outdated
Comment on lines +1339 to +1355
def factory(role):
if role == "linear_weight":
return HybridQuantizer(
rowwise_quantizer=_make_fp8_quantizer(),
columnwise_quantizer=_make_mxfp8_quantizer(),
)
if role == "linear_input":
return HybridQuantizer(
rowwise_quantizer=_make_fp8_quantizer(),
columnwise_quantizer=_make_nvfp4_quantizer(),
)
if role in ("linear_grad_output", "linear_grad_input"):
return HybridQuantizer(
rowwise_quantizer=_make_mxfp8_quantizer(),
columnwise_quantizer=_make_nvfp4_quantizer(),
)
return None

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is horrifying. Good test.

negvet and others added 10 commits April 6, 2026 10:26
Signed-off-by: Evgeny <etsykunov@nvidia.com>
Signed-off-by: Evgeny <etsykunov@nvidia.com>
Signed-off-by: Evgeny <etsykunov@nvidia.com>
Signed-off-by: Evgeny <etsykunov@nvidia.com>
Signed-off-by: Evgeny <etsykunov@nvidia.com>
Comment thread transformer_engine/pytorch/module/grouped_linear.py Outdated
Comment thread transformer_engine/pytorch/tensor/hybrid_tensor.py
negvet and others added 2 commits April 29, 2026 16:02
Signed-off-by: Evgeny <etsykunov@nvidia.com>
Comment thread transformer_engine/pytorch/tensor/storage/float8_tensor_storage.py
negvet added 3 commits May 13, 2026 12:34
Signed-off-by: Evgeny <etsykunov@nvidia.com>
Signed-off-by: Evgeny <etsykunov@nvidia.com>
Signed-off-by: Evgeny <etsykunov@nvidia.com>
@negvet
negvet requested a review from ksivaman as a code owner May 21, 2026 13:53
Comment thread transformer_engine/pytorch/tensor/float8_tensor.py
Comment on lines +27 to +30
# DCP serializes ``CustomRecipe`` via ``pickle``; closure-based qfactories
# (lambdas, inner functions referencing captured state) are not picklable,
# so the qfactory must live at module scope. See
# ``run_fsdp2_fused_adam.py::test_hybrid_dcp_output_parity``.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is potentially useful, but I don't think it is in the right place - shouldn't it be closer to the actual implementation?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Comment on lines +1177 to +1184
for param in model.parameters():
state = optimizer.state[param]
assert state["exp_avg"].dtype == torch.float32
assert state["exp_avg_sq"].dtype == torch.float32
if "master_param" in state:
assert state["master_param"].dtype == torch.float32

assert losses[-1] < losses[0], f"Loss did not decrease: {losses[0]:.4f} -> {losses[-1]:.4f}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not a very strict test, is there a way for us to do some numerical correctness comparisons?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enabled check for the monotonic loss decrease (still mostly sanity), and also enabled hybrid vs vanilla bitwise recipe comparizon, see e.g. test_fused_adam_hybrid_vs_base_recipe_parity.

Signed-off-by: Evgeny <etsykunov@nvidia.com>

@kwyss-nvidia kwyss-nvidia left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Evgeny for this expansive PR!

I'm excited to see the columnwise_source options in hybrid quantizer and that the edge cases for the FSDP protocol are considered and captured in the new tensor types.

LGTM!

assert copied.rowwise_usage is False
assert copied.columnwise_usage is True

def test_rowwise_dequantized_identity_columnwise_matches_rowwise(self, input_tensor):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the coverage for double quantization and the field so that the quantized tensor tracks describes columnwise source.

# ---------------------------------------------------------------------------


# Module-level qfactories (picklable, required for checkpoint serialization).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which qfactories go into the checkpoint serialization?

While it's useful to have provenance of how the checkpoint was created, does the pickling of qfactories mean that the resulting checkpoints won't be read by transformer engine's with the same classes for custom quantization.

Is there any way to override this requirement and load the checkpoint as BF16, ignoring the pickled qfactories?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I clarified the comment: the module-level qfactory is only needed so TE-to-TE quantized-param checkpoints have a stable importable reference for any pickled quantizer/recipe metadata.

For portability: if quantized_model_init is disabled, model weights are normal BF16 tensors and the CustomRecipe extra state is not needed for stateless recipes, so an external consumer can ignore TE _extra_state. With quantized_model_init, the model state_dict stores TE quantized tensor subclasses for any recipe, not just hybrid/CustomRecipe, so TE -> third-party runtime portability would need a separate high-precision/BF16 export path for quantized primary weights.

Would it be useful to plan that quantized_model_init BF16-weight state_dict/export support as a follow-up, or is running without quantized_model_init for portable checkpoints sufficient for your use case?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running without quantized_model_init is sufficient. I did not know about that option!

transa = layout[0] == "T"
transb = layout[1] == "T"

A = _materialize_high_precision(_unwrap_hybrid_A(A, layout))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming convention could be revisited. It can be interpreted easily but falsely to mean all quantized tensors will be materialized into high precision tensors. Perhaps "_unwrap_if_high_precision"?

@negvet negvet Jun 30, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced with _unwrap_identity_tensor, since we are doing isinstance(tensor, IdentityTensorStorage)


# Linear-only recipe (no attention quantization): the qfactory is the only knob.
recipe = CustomRecipe(qfactory=mxfp8_fwd_nvfp4_bwd_quantizer_factory)
with autocast(recipe=recipe):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pleasantly simple as an API. Thanks Evgeny.

``HybridQuantizer`` terms, that source choice is expressed with
``columnwise_source="rowwise_dequantized"``.

All non-weight roles keep the standard NVFP4 factory behavior, including RHT

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is useful. We have trained with an equivalent recipe for several experiments and I'm looking forward to trying this implementation.

# Return early if recipe state matches recipe
if self.fp8_meta_tensors_initialized:
recipe_state = self.fp8_meta[fp8_meta_tensor_key]
# Follow-up: Match built-in recipes by full config, not just RecipeState type, so

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this follow up liked an issue or this pull request ID, it would be easier to grep for all related follow ups.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create an umbrella tracker #3158 and referenced it

-------
MXFP8 forward plus high-precision backward from the rowwise-dequantized
forward value can be expressed as::

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the zoo, there's an example where the weight tensor is specialized with double quantization. It would be helpful to illustrate that it's possible to customize along the weight/activation/grad axis as well as the rowwise/colwise abstraction and reference the zoo.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, fixed 8c23fed

for sub in (self.rowwise_quantizer, self.columnwise_quantizer):
group = getattr(sub, "amax_reduction_group", None)
if group is not None:
return group

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguably, this should assert if there are two groups, they are consistent. Is that possible?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, fixed 8c23fed

Signed-off-by: Evgeny <etsykunov@nvidia.com>
negvet and others added 3 commits June 30, 2026 13:47
Signed-off-by: Evgeny <etsykunov@nvidia.com>
Signed-off-by: root <root@prenyx0286.a51.clusters.nvidia.com>
Comment thread transformer_engine/pytorch/tensor/storage/mxfp8_tensor_storage.py
Comment thread transformer_engine/pytorch/module/grouped_linear.py
# Hybrid (CustomRecipe) needs no SP amax-reduction setup today: its SP
# activations are gathered in high precision and re-quantized whole, so
# every rank already sees the same global amax.
# TODO(#3158): once native quantized all-gather lands (see

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you intend to close those TODOs?

@negvet negvet Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but in the follow-up PRs, see #3158. for now it falls back to high-precision AG

Comment on lines +305 to +317
if (
save_original_input
and backward_needs_input
and input_quantizer is not None
and not input_quantizer.allows_save_original_input_for_backward()
):
warnings.warn(
"Ignoring save_original_input=True because the input quantizer requires "
"the forward quantized activation for backward "
f"({input_quantizer}).",
stacklevel=2,
)
save_original_input = False

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this check be moved to the quantizer contruction rather than being called on every forward pass?
save_original_input is dependent on module options and recipe.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep this check at runtime for now. save_original_input depends on thiings that might be changed at runtime right?

Comment on lines +58 to +60
qkv_quantizer = recipe.qfactory(
QuantizerRole(module_type="dpa", tensor_type="qkv", name=dpa_name)
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not look safe (and also is wasteful) to call this every time. E.g. if the custom quantizer
is stateful or uses RNG etc. we would be redoing seeding of it every time potentially changing the
result depending on how many forwards of MHA we called.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. The QKV quantizer is now materialized once through DPA recipe state and reused. MHA no longer calls qfactory to probe it.

"""

_hp_data: Optional[torch.Tensor]
_quantizer: Optional[Quantizer]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_quantizer is already a field in the base class.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in c7e11d2

"""Return the held high-precision tensor (no-op dequantization)."""
if self._hp_data is None:
raise RuntimeError("IdentityTensorStorage has no data to dequantize")
if dtype is not None and self._hp_data.dtype != dtype:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about self._dtype?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. It now defaults to its nominal dtype

# Return early if recipe state matches recipe
if self.fp8_meta_tensors_initialized:
recipe_state = self.fp8_meta[fp8_meta_tensor_key]
# Follow-up (#3157): Match built-in recipes by full config, not just RecipeState type, so

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change "Follow-up" to "TODO"? It is more discoverable that way.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

return False


def _has_identity_quantizer_list(quantizers):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be called every time and instead we should only update this when the recipe changes.

Comment on lines +182 to +187
non_none = [q for q in quantizers if q is not None]
if not non_none:
return False
hybrid_count = sum(1 for q in non_none if isinstance(q, HybridQuantizer))
if hybrid_count == 0:
return False

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty convoluted and inefficient.

"""
from ..tensor.storage.hybrid_tensor_storage import HybridQuantizedTensorStorage as HybridStorage

if not all(isinstance(q, HybridQuantizer) for q in quantizers):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But you already check that in the function above, why checking it again?

from ...debug.pytorch.debug_quantization import DebugQuantizer
from ...debug.pytorch.debug_state import TEDebugState


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A general comment to this entire file: this introduces quite a lot of CPU overhead to this module.
We should just limit this to only allow the same quantizers everywhere rather than checking every
time.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General comment for the grouped_linear.py
This and the three related comments above are valid and resolved. See c7e11d2

if out_data is not None:
out_shape = out_data.size()
else:
out_shape = torch.empty(tuple(self.size()), device="meta").view(shape).shape

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh. That's pretty roundabout way of calculating that...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, take a look at the fix at c7e11d2

"""The sub-storage providing columnwise quantized data."""
return self._columnwise_storage

def update_usage(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Asking for a storage that does not exist should result in error.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a check for missing directions

Comment on lines +41 to +43
def _canonical_view_shape(input_shape: Iterable[int], shape: Iterable[int]) -> torch.Size:
"""Resolve PyTorch view shape syntax, including ``-1`` inference."""
return torch.empty(tuple(input_shape), device="meta").view(*shape).shape

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the same formula that was used in one of the other files. If useful, it should be somewhere where it could be used without rediscovering, but still I think it is pretty roundabout...

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally using torch.empty, view etc would introduce quite a bit of CPU overheads.. We can define a function to do manual size resolution in python that resolves -1, 0 etc.

Here something Gemini gave me

def get_actual_view_shape_full(input_shape: tuple, view_shape: tuple) -> tuple:
    # 1. First, resolve any '0' dimensions by copying from the input_shape
    resolved_zeros = []
    for i, dim in enumerate(view_shape):
        if dim == 0:
            if i >= len(input_shape):
                raise ValueError("Dimension '0' exceeds input shape dimensions")
            resolved_zeros.append(input_shape[i])
        else:
            resolved_zeros.append(dim)
            
    # 2. Calculate element totals
    total_elements = math.prod(input_shape)
    known_elements = math.prod(d for d in resolved_zeros if d != -1)
    
    # 3. Resolve the '-1' dimension if it exists
    if -1 in resolved_zeros:
        if resolved_zeros.count(-1) > 1:
            raise ValueError("Only one dimension can be -1")
        if total_elements % known_elements != 0:
            raise ValueError(f"Cannot reshape {input_shape} to {view_shape}")
            
        missing_dim = total_elements // known_elements
        return tuple(missing_dim if d == -1 else d for d in resolved_zeros)
        
    # 4. Strict validation if no -1 is used
    if total_elements != known_elements:
        raise ValueError(f"Cannot reshape {input_shape} to {view_shape}")
        
    return tuple(resolved_zeros)

@negvet negvet Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now float8 tensor and storage share the same better shaped utility, see c7e11d2

)

@classmethod
def __torch_dispatch__(cls, func, types, args, kwargs=None):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this function to be this big? We should be able to mostly just delegate to the held tensor, right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, delegated

Comment on lines +1905 to +1907
"instance or a QuantizerRequest. For an intentional no-op "
"(high-precision / unquantized) slot, return an "
"IdentityQuantizer instead of None."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could probably just allow None to mean the IdentityQuantizer here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not do that. Current behavior is explicit. Treating a factory return value of None as an Identity quantizer would silently broaden the custom recipe API and could hide a missing role. Also none quantizer has a semantic meaning in the modules, which is different from the IdentityQuantizer, right? I suggest Identity should be requested explicitly.

Comment on lines +41 to +43
def _canonical_view_shape(input_shape: Iterable[int], shape: Iterable[int]) -> torch.Size:
"""Resolve PyTorch view shape syntax, including ``-1`` inference."""
return torch.empty(tuple(input_shape), device="meta").view(*shape).shape

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally using torch.empty, view etc would introduce quite a bit of CPU overheads.. We can define a function to do manual size resolution in python that resolves -1, 0 etc.

Here something Gemini gave me

def get_actual_view_shape_full(input_shape: tuple, view_shape: tuple) -> tuple:
    # 1. First, resolve any '0' dimensions by copying from the input_shape
    resolved_zeros = []
    for i, dim in enumerate(view_shape):
        if dim == 0:
            if i >= len(input_shape):
                raise ValueError("Dimension '0' exceeds input shape dimensions")
            resolved_zeros.append(input_shape[i])
        else:
            resolved_zeros.append(dim)
            
    # 2. Calculate element totals
    total_elements = math.prod(input_shape)
    known_elements = math.prod(d for d in resolved_zeros if d != -1)
    
    # 3. Resolve the '-1' dimension if it exists
    if -1 in resolved_zeros:
        if resolved_zeros.count(-1) > 1:
            raise ValueError("Only one dimension can be -1")
        if total_elements % known_elements != 0:
            raise ValueError(f"Cannot reshape {input_shape} to {view_shape}")
            
        missing_dim = total_elements // known_elements
        return tuple(missing_dim if d == -1 else d for d in resolved_zeros)
        
    # 4. Strict validation if no -1 is used
    if total_elements != known_elements:
        raise ValueError(f"Cannot reshape {input_shape} to {view_shape}")
        
    return tuple(resolved_zeros)

raise NotImplementedError(
"FSDP2 is only supported for MXFP8Tensors with compact scales"
)
names = self.fsdp_buffer_fields()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to reuse fsdp extract buffers and fsdp_assign_gather for the current tensor class as well along with hybrid tensor(in this case mxfp8 and similarly for others)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense. I would make a standalone PR under #3158. I updated #3158 with this.

return dst

# ── FSDP2: new_zeros ─────────────────────────────────────────
if func == aten.new_zeros.default:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is risky if we use it for anything apart from FSDP2. Might make sense to zero out the substorages.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed!

# Factories stay small; these tests target TP/SP plumbing.


def _make_fp8_current_quantizer(*, fp8_dtype=tex.DType.kFloat8E4M3):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now recommend to use transformer_engine.pytorch.DType instead of tex.DType in TE. So we should change it here as well

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

negvet and others added 5 commits July 13, 2026 15:15
@negvet
negvet requested review from ptrendx and vthumbe1503 July 17, 2026 10:45
pre-commit-ci Bot and others added 3 commits July 17, 2026 10:46
@negvet

negvet commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

/te-ci pytorch L1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants